[SPARK-42538][CONNECT] Make `sql.functions#lit` function support more types #40143

LuciferYang · 2023-02-23T14:10:48Z

What changes were proposed in this pull request?

This pr aims add more types support of sql.functions#lit function, include:

Decimal
Instant
Timestamp
LocalDateTime
Date
Duration
Period
CalendarInterval

Why are the changes needed?

Make ·sql.functions#lit· function support more types

Does this PR introduce any user-facing change?

No

How was this patch tested?

Add new test
Manual checked new case with Scala-2.13

LuciferYang · 2023-02-23T14:26:47Z

Some other things to do, will continue tomorrow

hvanhovell · 2023-02-23T16:20:12Z

connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/functions.scala

    }
  }
+
+  private def literalToColumn(literal: Literal): Column = {


We don't need this. In fact for the next Spark release we will be removing the Catalyst dependency.

So, we should remove case v: Literal => literalToColumn(v) ?

Yeah we should. Sorry about that.

ok, let me update the code

hvanhovell · 2023-02-23T16:20:49Z

connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/functions.scala

      case v: Array[Byte] => createLiteral(_.setBinary(ByteString.copyFrom(v)))
      case v: collection.mutable.WrappedArray[_] => lit(v.array)
      case v: LocalDate => createLiteral(_.setDate(v.toEpochDay.toInt))
+      case v: UTF8String => createLiteral(_.setString(v.toString))


This is internal API. Can you remove it.

hvanhovell · 2023-02-23T16:22:53Z

@LuciferYang thanks for the PR! Which datatypes are we still missing? I think we still some collection support?

LuciferYang · 2023-02-23T16:27:34Z

I refer

spark/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/literals.scala

Lines 64 to 102 in 87e3d56

    
           def apply(v: Any): Literal = v match { 
        
             case i: Int => Literal(i, IntegerType) 
        
             case l: Long => Literal(l, LongType) 
        
             case d: Double => Literal(d, DoubleType) 
        
             case f: Float => Literal(f, FloatType) 
        
             case b: Byte => Literal(b, ByteType) 
        
             case s: Short => Literal(s, ShortType) 
        
             case s: String => Literal(UTF8String.fromString(s), StringType) 
        
             case s: UTF8String => Literal(s, StringType) 
        
             case c: Char => Literal(UTF8String.fromString(c.toString), StringType) 
        
             case ac: Array[Char] => Literal(UTF8String.fromString(String.valueOf(ac)), StringType) 
        
             case b: Boolean => Literal(b, BooleanType) 
        
             case d: BigDecimal => 
        
               val decimal = Decimal(d) 
        
               Literal(decimal, DecimalType.fromDecimal(decimal)) 
        
             case d: JavaBigDecimal => 
        
               val decimal = Decimal(d) 
        
               Literal(decimal, DecimalType.fromDecimal(decimal)) 
        
             case d: Decimal => Literal(d, DecimalType(Math.max(d.precision, d.scale), d.scale)) 
        
             case i: Instant => Literal(instantToMicros(i), TimestampType) 
        
             case t: Timestamp => Literal(DateTimeUtils.fromJavaTimestamp(t), TimestampType) 
        
             case l: LocalDateTime => Literal(DateTimeUtils.localDateTimeToMicros(l), TimestampNTZType) 
        
             case ld: LocalDate => Literal(ld.toEpochDay.toInt, DateType) 
        
             case d: Date => Literal(DateTimeUtils.fromJavaDate(d), DateType) 
        
             case d: Duration => Literal(durationToMicros(d), DayTimeIntervalType()) 
        
             case p: Period => Literal(periodToMonths(p), YearMonthIntervalType()) 
        
             case a: Array[Byte] => Literal(a, BinaryType) 
        
             case a: collection.mutable.WrappedArray[_] => apply(a.array) 
        
             case a: Array[_] => 
        
               val elementType = componentTypeToDataType(a.getClass.getComponentType()) 
        
               val dataType = ArrayType(elementType) 
        
               val convert = CatalystTypeConverters.createToCatalystConverter(dataType) 
        
               Literal(convert(a), dataType) 
        
             case i: CalendarInterval => Literal(i, CalendarIntervalType) 
        
             case null => Literal(null, NullType) 
        
             case v: Literal => v 
        
             case _ => 
        
               throw QueryExecutionErrors.literalTypeUnsupportedError(v) 
        
           }

the missing is case a: Array[_]

hvanhovell · 2023-02-23T16:46:11Z

A couple of things:

For Array I guess we can just make a nested call to lit for each element. No need to get CatalystTypeConverters involved.
Bonus points if you check if all elements are the same.
We do have to check what CatalystTypeConverters currently supports though. I think Map/Seq/Product support is also in there.
You may want to put this in a separate file.

hvanhovell · 2023-02-23T16:48:20Z

Oh and if it becomes too large I am fine with merging this first, and doing array in a follow-up.

LuciferYang · 2023-02-23T17:02:15Z

hmm... If I understand correctly, the current Literal does not support any collection type? Do we need to add some message types to support them?

spark/connector/connect/common/src/main/protobuf/spark/connect/expressions.proto

Lines 149 to 175 in 87e3d56

    
           message Literal { 
        
             oneof literal_type { 
        
               DataType null = 1; 
        
               bytes binary = 2; 
        
               bool boolean = 3; 
        
               int32 byte = 4; 
        
               int32 short = 5; 
        
               int32 integer = 6; 
        
               int64 long = 7; 
        
               float float = 10; 
        
               double double = 11; 
        
               Decimal decimal = 12; 
        
               string string = 13; 
        
               // Date in units of days since the UNIX epoch. 
        
               int32 date = 16; 
        
               // Timestamp in units of microseconds since the UNIX epoch. 
        
               int64 timestamp = 17; 
        
               // Timestamp in units of microseconds since the UNIX epoch (without timezone information). 
        
               int64 timestamp_ntz = 18; 
        
               CalendarInterval calendar_interval = 19; 
        
               int32 year_month_interval = 20; 
        
               int64 day_time_interval = 21; 
        
             }

hvanhovell · 2023-02-23T17:04:39Z

lol, no it does not. Let's just implement what we support, and do the rest in a different PR.

LuciferYang · 2023-02-23T17:05:15Z

OK

LuciferYang · 2023-02-23T17:09:00Z

Oh and if it becomes too large I am fine with merging this first, and doing array in a follow-up.

I hope we can merge this pr first if no other need to change. In addition, I need to go to bed as soon as possible. It's 1:00 in my time zone :)

hvanhovell · 2023-02-23T17:10:00Z

go to sleep!

LuciferYang · 2023-02-24T07:40:24Z

@hvanhovell Is there anything else can help Scala Client? @panbingkun told me that he also wanted to take some work related to connect.

hvanhovell · 2023-02-24T11:59:49Z

@LuciferYang @panbingkun that would be great! I will create an epic, with a bunch of todo's.

LuciferYang · 2023-02-24T14:46:11Z

Is there anything else need change this pr?

hvanhovell · 2023-02-24T18:09:45Z

@LuciferYang can you update your PR?

hvanhovell

LGTM

hvanhovell · 2023-02-24T18:49:00Z

@LuciferYang @panbingkun I created an epic with a bunch of things you can pick up: https://issues.apache.org/jira/browse/SPARK-42554

amaliujia · 2023-02-24T19:01:47Z

LGTM but please rebase this PR to solve conflict.

hvanhovell · 2023-02-25T18:08:33Z

Merging to master/3.4. Thanks!

… types ### What changes were proposed in this pull request? This pr aims add more types support of `sql.functions#lit` function, include: - Decimal - Instant - Timestamp - LocalDateTime - Date - Duration - Period - CalendarInterval ### Why are the changes needed? Make ·sql.functions#lit· function support more types ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? - Add new test - Manual checked new case with Scala-2.13 Closes #40143 from LuciferYang/functions-lit. Authored-by: yangjie01 <yangjie01@baidu.com> Signed-off-by: Herman van Hovell <herman@databricks.com> (cherry picked from commit 2a4aab7) Signed-off-by: Herman van Hovell <herman@databricks.com>

… types ### What changes were proposed in this pull request? This pr aims add more types support of `sql.functions#lit` function, include: - Decimal - Instant - Timestamp - LocalDateTime - Date - Duration - Period - CalendarInterval ### Why are the changes needed? Make ·sql.functions#lit· function support more types ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? - Add new test - Manual checked new case with Scala-2.13 Closes apache#40143 from LuciferYang/functions-lit. Authored-by: yangjie01 <yangjie01@baidu.com> Signed-off-by: Herman van Hovell <herman@databricks.com> (cherry picked from commit 2a4aab7) Signed-off-by: Herman van Hovell <herman@databricks.com>

LuciferYang added 4 commits February 23, 2023 19:30

Except Array[_] and Literal

ca0e043

Literal

32208bf

add test

fbb2605

add short

ccf8eba

LuciferYang marked this pull request as draft February 23, 2023 14:10

github-actions bot added CONNECT SQL labels Feb 23, 2023

reorder

8c7a503

LuciferYang marked this pull request as ready for review February 23, 2023 14:26

LuciferYang added 3 commits February 23, 2023 23:39

change timestamp case

68c7edb

Merge branch 'upmaster' into functions-lit

fd12bf0

format

c3a1e49

hvanhovell reviewed Feb 23, 2023

View reviewed changes

LuciferYang added 2 commits February 24, 2023 00:52

remove Literal and UTF8String

9f2f810

remove empty line and import

aafb2ae

hvanhovell approved these changes Feb 24, 2023

View reviewed changes

Merge branch 'upmaster' into functions-lit

b10150e

hvanhovell closed this in 2a4aab7 Feb 25, 2023

[SPARK-42538][CONNECT] Make sql.functions#lit function support more types #40143

[SPARK-42538][CONNECT] Make sql.functions#lit function support more types #40143

Uh oh!

Conversation

LuciferYang commented Feb 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

LuciferYang commented Feb 23, 2023

Uh oh!

hvanhovell Feb 23, 2023

Choose a reason for hiding this comment

Uh oh!

LuciferYang Feb 23, 2023

Choose a reason for hiding this comment

Uh oh!

hvanhovell Feb 23, 2023

Choose a reason for hiding this comment

Uh oh!

LuciferYang Feb 23, 2023

Choose a reason for hiding this comment

Uh oh!

hvanhovell Feb 23, 2023

Choose a reason for hiding this comment

Uh oh!

LuciferYang Feb 23, 2023

Choose a reason for hiding this comment

Uh oh!

hvanhovell commented Feb 23, 2023

Uh oh!

LuciferYang commented Feb 23, 2023

Uh oh!

hvanhovell commented Feb 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hvanhovell commented Feb 23, 2023

Uh oh!

LuciferYang commented Feb 23, 2023

Uh oh!

hvanhovell commented Feb 23, 2023

Uh oh!

LuciferYang commented Feb 23, 2023

Uh oh!

LuciferYang commented Feb 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hvanhovell commented Feb 23, 2023

Uh oh!

LuciferYang commented Feb 24, 2023

Uh oh!

hvanhovell commented Feb 24, 2023

Uh oh!

LuciferYang commented Feb 24, 2023

Uh oh!

hvanhovell commented Feb 24, 2023

Uh oh!

hvanhovell left a comment

Choose a reason for hiding this comment

Uh oh!

hvanhovell commented Feb 24, 2023

Uh oh!

amaliujia commented Feb 24, 2023

Uh oh!

hvanhovell commented Feb 25, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

[SPARK-42538][CONNECT] Make `sql.functions#lit` function support more types #40143

[SPARK-42538][CONNECT] Make `sql.functions#lit` function support more types #40143

LuciferYang commented Feb 23, 2023 •

edited

Loading

hvanhovell commented Feb 23, 2023 •

edited

Loading

LuciferYang commented Feb 23, 2023 •

edited

Loading